Harden reward JavaScript placeholder evaluation - #293
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2be840bea1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abe43c0d7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4373528b33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 123177fe21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3487a9d0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26505fd6a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7128d79b3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddd05e2a5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17fc31bc92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1abf4d51e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ("=?:,;".indexOf(current) >= 0) { | ||
| return false; |
There was a problem hiding this comment.
When a valid class declaration includes a comment containing punctuation, such as class /* = */ X {} /[']/.test('x'); %untrusted%, this backward scan encounters the comment's = and classifies the body as an expression brace. The following regex is then treated as division, its quotes corrupt placeholder context, and an untrusted value such as Bukkit.dispatchCommand(Console, "op attacker") is emitted directly into executable source. Fresh evidence beyond the existing class-declaration finding is that comment contents are interpreted as declaration-boundary tokens; skip lexical comments while locating class.
Useful? React with 👍 / 👎.
| if (tokenEnd < 0 || !Character.isJavaIdentifierPart(script.charAt(tokenEnd))) { | ||
| return false; |
There was a problem hiding this comment.
When division follows a function expression whose keyword is separated from its parameters by a comment, as in var allowed=false,x=function /* comment */ () {} / 2; '%untrusted%'; allowed, tokenEnd points at the comment terminator and this return prevents recognition of the function expression. Its body is consequently treated as a statement block, the division slash opens a false regex context, and a value such as '; allowed=true; ' breaks out of the later quoted placeholder and changes the result. Fresh evidence beyond the existing function-expression finding is the valid comment-separated parameter-list form.
Useful? React with 👍 / 👎.
| import java.util.regex.Pattern; | ||
|
|
||
| final class JavascriptPlaceholderParser { | ||
| private static final Pattern PLACEHOLDER_PATTERN = Pattern.compile("%([^%]+)%|(?<!\\$)\\{([^{}%]+)\\}"); |
There was a problem hiding this comment.
When an expression has a modulo operator before a percent placeholder, for example 5 % 2 == 1 && %permission_result%, this pattern matches % 2 == 1 && % first. Because that text is unresolved, the matcher advances past the real placeholder's opening %, leaving %permission_result% unresolved and usually making the requirement invalid JavaScript. Match percent placeholders using their valid placeholder-name grammar rather than pairing arbitrary percent signs.
Useful? React with 👍 / 👎.
|
Superseded by a smaller redesign. The lexer-based approach grew too large because it tried to preserve arbitrary placeholder placement across JavaScript syntax. Closing this PR and replacing it with a clean implementation where placeholder results are treated only as bound expression values and normal placeholder expansion cannot manufacture executable JavaScript markers. |
1 participant
Summary
[Javascript=...]blocks before normal placeholder expansion so replacement values cannot create new executable blocksJavascriptExpression,Javascripts, and conditionalJavascript.Expressionreward paths to use the safe preparation pathSecurity context
This addresses the shared trust-boundary issue behind these findings:
%displayname%[Javascript=...]blocksOperator-authored JavaScript remains supported; data produced by placeholder expansion is kept as data rather than being allowed to change the program structure.